-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Add passkey tests verifying WebAuthn conformance #62441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
cc9262a
to
bdb2262
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR adds CI-compatible tests that verify conformance with the WebAuthn specification, at least to the degree that we currently support (e.g., there are no attestation statement verification tests). Instead of validating passkey functionality in an E2E manner like the conformance testing tool (we already have some coverage in existing E2E tests), these tests directly validate the DefaultPasskeyHandler implementation in-memory. This is a simpler approach that gives us full control over test input data that would normally be generated by the browser and allows us to validate implementation-specific behavior that the official conformance testing tool cannot.
This looks really good. I agree with the in-memory approach. Other than the attestation statement verification tests, is there any other coverage that we're missing? Feel free to ignore my comment about the file structure. Only change it if you think it's better.
} | ||
|
||
// Represents a test scenario for a passkey operation (attestation or assertion) | ||
private abstract class PasskeyTestBase<TResult> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see someone else is not a fan of XUnit test fixtures either 😆. Would it be simpler to put the helper types like this and CredentialKeyPair in their own files in a new Identity.Test/Passkeys subfolder rather than having a huge DefaultPasskeyHandlerTest split up over four files?
I'm fine with keep the attestation and authentication tests in separate files, but I think they should have different class names. That probably means a lot of private stuff will need to be made internal/public that won't be needed elsewhere, but at least it'd all be the same namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be simpler to put the helper types like this and CredentialKeyPair in their own files in a new Identity.Test/Passkeys subfolder rather than having a huge DefaultPasskeyHandlerTest split up over four files?
It might be - I did at one point try moving some of these helpers out into non-nested types, but that was at the same time I attempted an insane refactor that abstracted similar tests between attestation and assertion into a common base test class, which ended up being a bad idea. I'll try "just" moving these into separate files and having attestation and assertion tests be in separate classes to see if that works better 🙂
Not that I'm aware of - I think these tests cover everything else that the conformance testing tool checks for (plus things it doesn't/couldn't check for, such as handling incorrectly formatted creation/request options, attempting to log in with a credential ID not associated with the user identified prior to the login ceremony, etc.). |
Add passkey tests verifying WebAuthn conformance
Adds tests to verify that the ASP.NET Core Identity passkey implementation conforms to the WebAuthn standard.
Description
We have an existing sample project in this repo (
IdentitySample.PasskeyConformance
) that can be run against the official FIDO Conformance Testing tools to verify that our implementation of passkeys is conformant with the WebAuthn specification. However, it's currently not possible to run this tool in CI, as the tool is not open source nor does it have a CLI interface.This PR adds CI-compatible tests that verify conformance with the WebAuthn specification, at least to the degree that we currently support (e.g., there are no attestation statement verification tests). Instead of validating passkey functionality in an E2E manner like the conformance testing tool (we already have some coverage in existing E2E tests), these tests directly validate the
DefaultPasskeyHandler
implementation in-memory. This is a simpler approach that gives us full control over test input data that would normally be generated by the browser and allows us to validate implementation-specific behavior that the official conformance testing tool cannot.This PR also extends existing functional tests to cover passkey functionality:
SignInManager
testsUserManager
testsFixes #62440